Skip to content

Conversation

@artursarlo
Copy link

Fix: Enable RQL filter parsing for GET /api/v1/metrics/lasthtml endpoint

Problem

The GET /api/v1/metrics/lasthtml endpoint was not properly processing RQL (Resource Query Language) filter parameters, causing filters like hostname, container name, and other criteria to be ignored in database queries.

Root Cause:
The GetLastHTML handler was passing nil as the RQL parser to parseParams(), which meant that filter parameters in the request were not being parsed and converted to SQL WHERE conditions.

// Before (broken)
params, query, err := parseParams(common.MetricsLastHTMLParams{}, nil, c)

When parser == nil, the parseParams function skips RQL filter processing, resulting in:

  • Filter parameters from requests being ignored
  • Empty SQL WHERE conditions being generated
  • Queries returning results without applying user-specified filters

Solution

Changed the GetLastHTML handler to use MetricsQueryParser instead of nil, enabling proper RQL filter parsing:

// After (fixed)
params, query, err := parseParams(common.MetricsLastHTMLParams{}, MetricsQueryParser, c)

This ensures that:

  • RQL filters in requests are properly parsed into SQL conditions
  • Database queries include appropriate WHERE clauses
  • The endpoint respects user-specified filtering criteria

Impact

Before: Requests like /api/v1/metrics/lasthtml?filter={"HostName":{"$eq":"server1"}} would ignore the hostname filter and return results from all hosts.

After: The same request now properly filters results to only include data from "server1".

Testing

This fix aligns the GetLastHTML handler with other metrics endpoints that already use MetricsQueryParser for consistent filter behavior across the API.

Files Changed

  • src/gprofiler_flamedb_rest/handlers/handlers.go: Updated GetLastHTML handler to use MetricsQueryParser

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update

@mlim19 mlim19 requested a review from skamerintel November 21, 2025 01:57
@mlim19
Copy link
Contributor

mlim19 commented Nov 21, 2025

This looks good to me. @skamerintel, can you approve this if you are okay?

Copy link
Contributor

@skamerintel skamerintel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix aligns GetLastHTML with the other metrics endpoints and fixes bug with correctly RQL filter parsing.

Approved.

@artursarlo
Copy link
Author

I don't have permissions to merge, can you trigger it @mlim19 ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants